Okay, I've almost got this sorted. There remain a small set of failing tests due to nested scopes and associated values, which think I can tackle. This has required an API change in how we create nested scopes from a common sealed parent. I'm going to try and distill that down to a simple API, maybe something like public Toplevel createIsolate() and public TopLevel.createIsolate(ScriptableObject customGlobal) or something like that. Since this would change our documented API we would need to make a change to the docs as well at some point.

One concern I have is that it's not great how in so many places you have to do:

        if (scope instanceof TopLevel) {
            scope = ((TopLevel) scope).getGlobalThis();
        }

I am unsure on how we could improve it, but it's really diffused basically everywhere we have Function.call. Feels easy to forget in some place, especially for new code added in the future.

I am not sure why you are doing all the delegation in TopLevel to the globalThis. I have some ideas, but if you could draw a diagram of the scope structure, it would be very helpful for me.

@andreabergia I agree that I don't really like the tests for top level that are now scattered through several bits of initialisation code. The original reason to not force the top scope to be a TopLevel was that we had historically allowed custom objects to be used, but I think the separation here allows users to have a custom object as their global this and still force the top level scope to be an actual TopLevel. This would certainly help make behaviour consistent with respect to generator functions and other objects whose prototypes are never created as normal properties of the global scope.

I've prototyped this by doing it in stages

1. Making the object we create if null is provided as the scope.
2. Throwing an error if we are passed something that isn't a TopLevel as the scope
3. Then changing the type.

I'm currently on step 2. and have had make small adjustments to four tests, and have another four which seem to need similar fixes. @gbrail how would you feel about this API change?

Okay, I've moved all the initialisation stuff over to requiring a TopLevel, and it wasn't a big change really. I still have one failing test case due to serialisation and class caches. I'll investigate that over the weekend.

There are still couple of explicit checks for TopLevel which I'm also going to try and work through to see which can reasonably be removed without causing too much disruption.

Well, that went well. 😆 so it turns out that even if we force the creation of all top level scopes to be actual TopLevels that isn’t enough because of two things

1. Objects that are created to be scopes but have their parent scope set to null.
2. Places where we manage to swap the scope and the global this In our code.

The serialisation problem is similar in that the relationship between the top level scope and the global this has been fully set up at the point when something is trying to rely on that.

I’m setting this back to draft until I can track down those problems.

Had a burst of inspiration this morning and got this down to about 30 failing test cases. I think all the remaining cases are actual bugs where we ended up with TypeInfoFactory cached against random objects that hadn't had their scopes set up yet. There's a couple of APIs around defining properties which need fixing, it turns out I even did those fixes on my prototype branch for scopes, so I'll try and bring those across to here.

The serialisation problem is occurring because of the way I handled associated values on the top level scope. To enable the creation of isolates I put the values on the global object rather than the scope, and relied on being able to reach that object before it has been associated with top level scope, so the lookup breaks.

I think the thing to do here is to delegate associated values explicitly when creating isolates. I think two top level scopes created from a sealed shared scope should be guaranteed to share the same type cache.

I feel like I'm falling behind here, and apologies -- but it's a testament to the great work you're all doing.

I want to think more carefully about what we're doing to backward compatibility with all this stuff. I've never been super happy about the whole "parent scope" thing, but it's a fundamental part of Rhino and creating a "sealed shared scope" is used by a lot of examples and docs and probably real projects.

Can we document somewhere what we're proposing that will require people to change their current code, and then make a plan to stage it in upcoming releases?

Between this and other things we're working on, as well as the increased velocity of work on the project, it may indeed be time for a 1.9.0 or even a 2.0 with some reasonable changes to backward compatibility in favor of spec compliance.

Just a possible heads-up when doing stuff in the area of sealed/shared scopes: #1085

@gbrail Strongly agree. I think we're approaching the point where we want to do a release prior to any significant API change, and then work to get all those API changes into a 2.0.0 release. I think it might be worth it to create a 2.0 feature branch which we use for this and regularly rebase it on top of master.

I understand why people want to create a sealed parent object as it is a reasonably simple way to avoid repeated initialisation and prevent accidental modification, and we should absolutely ensure there is an API for doing so. I think that can be done, ad we can make it make it easier to use than the current setPrototype/setParentScope dance. That would give people a simple way to do this which we could encourage them to move to, and would hide most of the specifics allowing us to refactor things internally.

Okay. Looks like there is a small change required for some Android tests and a little tidy up required. I'll try and do that over the weekend.

Puh, this is huge (and i have to attend a workshop the next days). Will try it next week - hope that is ok

And a question - my top level scope is the window (or the globalworker)

• Window extends EventTarget
• EventTarget extends HtmlUnitScriptable
• HtmlUnitScriptable extends ScriptableObject

Do i have to make Window extending TopLevel?

Your Window should become the globalThis of a TopLevel, so you should be able to do something like var scope = new TopLevel(new Window) or use the createIsolate API if you want to make a sealed parent.